-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
allow s2s bidders call pbs without need of a client adapter file #2704
Conversation
While it's ok for s2s to not have a client-side adapter, vendors and pubs should realize it's potentially a disadvantage to not include the client in the PBJS package. Even if the main auction logic happens on the server side, there may be client-side functions -- currently there's usersync, but I believe there was another feature discussed. We'll need to update the S2S instructions on prebid.org. |
@bretg Thanks for the feedback. I'll put together a docs PR soon. |
To reviewers - I realized there is a workflow regarding aliasing bidders that I didn't incorporate when putting these changes together. Please wait from reviewing until I compile these additional changes and update the PR. |
I pushed in the updates to support aliasing bidders. |
Created docs PR for these updates: prebid/prebid.github.io#834 |
We are waiting for this PR merge to start our integration testing for S2S.
Can you please let us know what is the ETA for merging this PR?
…On Tue, Jun 12, 2018 at 7:54 AM, jsnellbaker ***@***.***> wrote:
Created docs PR for these updates: prebid/prebid.github.io#834
<prebid/prebid.github.io#834>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2704 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AlkKipYn-Nc2FkzfdukdRFkBAG_8voI-ks5t79Y8gaJpZM4UgnHj>
.
|
const videoBid = bids.some(bidResponse => bidResponse.bid.mediaType === 'video'); | ||
const cacheEnabled = config.getConfig('cache.url'); | ||
|
||
// video bids with cache enabled need to be cached first before they are considered done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a separate change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No those changes are part of this PR. They are needed to ensure pbs video bids that are using prebid cache are properly used in the auction. Without this change; the adapter calls the done()
before the caching process has finished which ends the auction prematurely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some extra commits got added to this diff by mistake. Can you fix to get a clean diff for this change only?
*@jsnellbaker <https://github.com/jsnellbaker> *
What is the status of this PR? One of our publisher is ready to start
testing our brightroll S2S adapter. Can they do that with existing
Prebid.js they have already? Or is this PR a blocker for testing? If so,
when can I expect these changes? Please advise.
…On Mon, Jun 18, 2018 at 11:15 AM, jsnellbaker ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In modules/prebidServerBidAdapter.js
<#2704 (comment)>:
> @@ -734,7 +735,13 @@ export function PrebidServer() {
utils.logError('error parsing response: ', result.status);
}
- done();
+ const videoBid = bids.some(bidResponse => bidResponse.bid.mediaType === 'video');
+ const cacheEnabled = config.getConfig('cache.url');
+
+ // video bids with cache enabled need to be cached first before they are considered done
No those changes are part of this PR. They are needed to ensure pbs video
bids that are using prebid cache are properly used in the auction. Without
this change; the adapter calls the done() before the caching process has
finished which ends the auction prematurely.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2704 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AlkKiu___r8XEgyZN5Wwb4x3tQqdCKMeks5t9-5OgaJpZM4UgnHj>
.
|
@smithaammassamveettil |
@mkendall07 I resolved the conflicts. Please take a look over again when you have the chance. Thanks. |
Type of change
Description of change
To add support for the request noted in #2659
This will allow a pbs bidder to make a request through prebid.js without needing a corresponding client-side adapter file setup in prebid.js.
These changes are mainly to support video-type requests through this type of workflow.